Fix cloudpickle incompatibilities on early Python 3.5 versions #361

pierreglaser · 2020-04-28T20:10:54Z

Closes #360 . cloudpickle 1.4.0 is not compatible with early Python 3.5 versions.
This should fix it.

Note that I did not set up any CI for Python 3.5.[0-2], I simply tested it on my machine using fresh conda envs.

@vedran If you have some time, could you tell me if this branch fixes the problems that made you create #360?

I would be tempted to release a bugfix version by tonight since this bug completely breaks cloudpickle on Python 3.5.

codecov · 2020-04-28T20:12:07Z

Codecov Report

Merging #361 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #361   +/-   ##
=======================================
  Coverage   92.95%   92.95%           
=======================================
  Files           2        2           
  Lines         809      809           
  Branches      164      164           
=======================================
  Hits          752      752           
  Misses         29       29           
  Partials       28       28

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e58c34b...e58c34b. Read the comment docs.

ogrisel

First batch of reviews:

cloudpickle/cloudpickle.py

pierreglaser · 2020-04-28T20:42:24Z

rebased.

…le into cloudpickle-py350

ogrisel · 2020-04-28T21:03:04Z

Maybe it's not worth fighting with the CI too much to test Python 3.5.0... I will test this PR one more time locally.

pierreglaser · 2020-04-28T21:03:21Z

Cool.

ogrisel · 2020-04-28T21:06:22Z

How did you install Python 3.5.0 to run your tests? From source? I cannot find it with conda.

pierreglaser · 2020-04-28T21:06:22Z

let me know when if it works locally - if so, I'll clean up the CI additions in this PR.

pierreglaser · 2020-04-28T21:07:06Z

How did you install Python 3.5.0 to run your tests? From source? I cannot find it with conda.

Using miniconda2. But actually miniconda3 works too:

conda create -y -n c3tmp35 python=3.5.0

ogrisel · 2020-04-28T21:25:00Z

Ok I managed to run the tests of this PR successfully using python:3.5.0 from docker.

ogrisel · 2020-04-28T21:26:42Z

From your last CI run:

ERROR: Package 'cloudpickle' requires a different Python: 2.7.17 not in '>=3.5'

That's an interesting error message ;) I am not sure what it means.

Anyways, feel free to rollback the CI changes and merge this PR to make the release.

pierreglaser · 2020-04-28T21:28:20Z

Great. I'm quickly testing 3.5.1 and 3.5.2 while I'm at it, and i'll merge.

ogrisel · 2020-04-28T21:51:10Z

Actually I made a mistake when testing with Python 3.5.0. I had forgotten to install the typing_extensions package. If I do so I get the following failure:

___________________________________________________________________ CloudPickleTest.test_generic_extensions ____________________________________________________________________

self = <tests.cloudpickle_test.CloudPickleTest testMethod=test_generic_extensions>

    def test_generic_extensions(self):
        typing_extensions = pytest.importorskip('typing_extensions')
    
        objs = [
            typing_extensions.Literal,
            typing_extensions.Final,
            typing_extensions.Literal['a'],
            typing_extensions.Final[int],
        ]
    
        for obj in objs:
            depickled_obj = pickle_depickle(obj, protocol=self.protocol)
>           assert depickled_obj == obj

tests/cloudpickle_test.py:2137: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/local/lib/python3.5/site-packages/typing_extensions.py:653: in __eq__
    if not isinstance(other, Literal):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = typing_extensions.Literal, obj = typing_extensions.Literal

    def __instancecheck__(self, obj):
>       raise TypeError("Literal cannot be used with isinstance().")
E       TypeError: Literal cannot be used with isinstance().

/usr/local/lib/python3.5/site-packages/typing_extensions.py:624: TypeError

pierreglaser · 2020-04-28T21:52:53Z

What kind of can of worms is this.....

TypeVar objects are not weakreferable in early Python 3.5

pierreglaser · 2020-04-28T22:38:50Z

The test suite passes locally on cloudpickle 3.5.[0-6] for me. 3.5.9 (the one used by the CI) passes too. miniconda does not seem to have 3.5.[7-8] so it's harder for me to test those two versions.

Let's sleep on it until tomorrow morning and then release.

ogrisel · 2020-04-29T06:17:41Z

ogrisel · 2020-04-29T06:29:52Z

For your information, here is the command line I use to run the tests with docker:

docker run --rm -v `pwd`:/io -ti python:3.5.2 \
    bash -c "cd /io/ && pip install -e . psutil pytest typing_extensions && pytest -v"

ogrisel

I think we should restore TypeVar tracking, at least for recent Python versions (>= 3.6).

cloudpickle/cloudpickle.py

ogrisel · 2020-04-29T06:40:30Z

cloudpickle/cloudpickle.py

        name, *constraints, bound=bound,
        covariant=covariant, contravariant=contravariant
    )
-    return _lookup_class_or_track(class_tracker_id, tv)


Why don't you track TypeVar defintions anymore? This seems unrelated to the change to support early 3.5.x.

Doesn't this change break ant test?

Edit: I see you removed test_pickle_dynamic_typevar_tracking.

Also won't this break unpickling objects pickled with cloudpickle 1.4.0?

TyperVar instances are not weakreferable in Python 3.5.3..

Also won't this break unpickling objects pickled with cloudpickle 1.4.0?

We can always restore the class_tracker_id for backward compat.

TyperVar instances are not weakreferable in Python 3.5.3..

I re-ran the tests with old Python 3.5.x and they pass...

Hum you are right they fail just for 3.5.3 ...

ok let me do a workaround for 3.5.3.

ogrisel · 2020-04-29T07:30:43Z

Ok I re-enabled dynamic TypeVar tracking and the tests still pass for all the 3 oldest versions of 3.5.x. Will try others in the 3.5.3-3.5.9 range at random.

pierreglaser · 2020-04-29T07:33:49Z

Ok I re-enabled dynamic TypeVar tracking and the tests still pass for all the 3 oldest versions of 3.5.x

I'm not sure we can. See #361 (comment)

ogrisel

Some more comments:

tests/cloudpickle_test.py

ogrisel

LGTM now! Thanks very much @pierreglaser.

ogrisel reviewed Apr 28, 2020

View reviewed changes

cloudpickle/cloudpickle.py Outdated Show resolved Hide resolved

cloudpickle/cloudpickle.py Outdated Show resolved Hide resolved

cloudpickle/cloudpickle.py Outdated Show resolved Hide resolved

pierreglaser added 7 commits April 28, 2020 22:40

fix basic incompatibilities with old Python3.5

fcbce90

fix pickling of type hints in Python 3.5.[0-2]

3fa3227

modify typing test to account for Python 3.5.[0-2]

732343e

fixup! fix pickling of type hints in Python 3.5.[0-2]

3adc5be

fixup! fix pickling of type hints in Python 3.5.[0-2]

d2b8aa6

update changelog

96c1d2f

fix mistake in parametrized type hint checker

7996513

pierreglaser force-pushed the cloudpickle-py350 branch from da2631e to 7996513 Compare April 28, 2020 20:42

pierreglaser changed the title ~~Cloudpickle py350~~ Fix cloudpickle incompatibilities on early Python 3.5 versions Apr 28, 2020

pierreglaser and others added 7 commits April 28, 2020 22:45

try to test cloudpickle agains Python 3.5.0 in CI

f164a29

try to use conda python in the CI

a7e521f

Merge branch 'master' into cloudpickle-py350

2c13f90

fixup! try to use conda python in the CI

4af5e8d

Merge branch 'cloudpickle-py350' of github.com:pierreglaser/cloudpick…

47d981a

…le into cloudpickle-py350

debug miniconda

2c9e644

fixup! debug miniconda

d5e0586

fixup! fixup! debug miniconda

f50f87f

pierreglaser added 5 commits April 28, 2020 23:08

fixup! fixup! debug miniconda

726d86d

fixup! fixup! fixup! debug miniconda

0d834ce

a few last tries

405b99a

fixup! a few last tries

b1eca25

I think I get it

abd46d7

ogrisel approved these changes Apr 28, 2020

View reviewed changes

rollback CI changes

1fb09a0

pierreglaser added 6 commits April 28, 2020 23:56

fix some Python version check

736127c

this cannot be done using version checks

6e3621d

this cannot be done using version checks

ec41dee

fix typing_extension tests for early Python 3.5

ae643fb

revert TypeVar tracking

a9dadd6

TypeVar objects are not weakreferable in early Python 3.5

fix sys version checks

acd23e4

add a few explanations

e573c37

ogrisel reviewed Apr 29, 2020

View reviewed changes

ogrisel and others added 2 commits April 29, 2020 09:13

Update cloudpickle/cloudpickle.py

c9c8b46

Re-enable dynamic TypeVar tracking

cf9e42e

ogrisel reviewed Apr 29, 2020

View reviewed changes

tests/cloudpickle_test.py Outdated Show resolved Hide resolved

tests/cloudpickle_test.py Outdated Show resolved Hide resolved

tests/cloudpickle_test.py Outdated Show resolved Hide resolved

tests/cloudpickle_test.py Show resolved Hide resolved

ogrisel and others added 3 commits April 29, 2020 09:35

Apply suggestions from code review

8c030a3

be more explicit when testing Final/Literal

6359bc5

Workaround for Python 3.5.3

e58c34b

ogrisel approved these changes Apr 29, 2020

View reviewed changes

ogrisel merged commit 0dd91b4 into cloudpipe:master Apr 29, 2020

jburnim mentioned this pull request Jul 6, 2020

Incompatibility with cloudpickle==1.5.0 tensorflow/probability#991

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cloudpickle incompatibilities on early Python 3.5 versions #361

Fix cloudpickle incompatibilities on early Python 3.5 versions #361

pierreglaser commented Apr 28, 2020

codecov bot commented Apr 28, 2020 •

edited

Loading

ogrisel left a comment

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

pierreglaser commented Apr 28, 2020 •

edited

Loading

ogrisel commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 29, 2020 •

edited

Loading

ogrisel commented Apr 29, 2020

ogrisel left a comment

ogrisel Apr 29, 2020

ogrisel Apr 29, 2020

pierreglaser Apr 29, 2020

pierreglaser Apr 29, 2020

ogrisel Apr 29, 2020

ogrisel Apr 29, 2020

ogrisel Apr 29, 2020

ogrisel commented Apr 29, 2020

pierreglaser commented Apr 29, 2020

ogrisel left a comment

ogrisel left a comment

Fix cloudpickle incompatibilities on early Python 3.5 versions #361

Fix cloudpickle incompatibilities on early Python 3.5 versions #361

Conversation

pierreglaser commented Apr 28, 2020

codecov bot commented Apr 28, 2020 • edited Loading

Codecov Report

ogrisel left a comment

Choose a reason for hiding this comment

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

pierreglaser commented Apr 28, 2020 • edited Loading

ogrisel commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

pierreglaser commented Apr 28, 2020

ogrisel commented Apr 29, 2020 • edited Loading

ogrisel commented Apr 29, 2020

ogrisel left a comment

Choose a reason for hiding this comment

ogrisel Apr 29, 2020

Choose a reason for hiding this comment

ogrisel Apr 29, 2020

Choose a reason for hiding this comment

pierreglaser Apr 29, 2020

Choose a reason for hiding this comment

pierreglaser Apr 29, 2020

Choose a reason for hiding this comment

ogrisel Apr 29, 2020

Choose a reason for hiding this comment

ogrisel Apr 29, 2020

Choose a reason for hiding this comment

ogrisel Apr 29, 2020

Choose a reason for hiding this comment

ogrisel commented Apr 29, 2020

pierreglaser commented Apr 29, 2020

ogrisel left a comment

Choose a reason for hiding this comment

ogrisel left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 28, 2020 •

edited

Loading

pierreglaser commented Apr 28, 2020 •

edited

Loading

ogrisel commented Apr 29, 2020 •

edited

Loading